-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try: Cover position fix round 2 #28653
Conversation
Size Change: +67 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the issue in my testing.
I don't think I'm the best person to review this. The hardcoded pixel dimensions concern me a bit, but this is certainly better than the embed not showing at all.
Thanks for working on it!
Cherry-picked to |
Also cherry-picked to |
Does this fix #28640? Can we close that issue? |
Yep, thank you noisy socks! |
Arguably better alternative to #28650, but please see that PR as well for a larger explainer on what the fundamental issue is.
Go there now, then come back and read the followup below. I'll wait.
...
Okay so the matrix aligner in the Cover block collapses every child block to its minimum dimensions so that it can align them to the corners. Which is problematic for responsive embeds, because they can collapse to 0x0.
This PR fixes that by assigning a minimum width/height to embeds:
But this opens questions:
initial
width and heights when aligned?This PR works as a baseline, but I'm not sure what the best next step. Feel free to commandeer this PR.